Refactor tile coverage to support level of detail #16335

mpulkki-mapbox · 2020-03-23T17:03:42Z

This pull request implements a more versatile logic for finding visible tiles on the screen. Intersection tests have been extended to 3D space by using bounding volumes for tiles and camera instead of projecting everything to zero elevation plane. Tile pyramid is traversed depth-first while preserving distances to tiles allowing an easy support for a level-of-detail. Even though the distance based lod is supported by this implementation, this PR has no visible effect for the user. Level of detail logic is deactive when pitch is <= 60 degrees.

The native implementation follows the javascript one (mapbox/mapbox-gl-js#8975) with few exceptions:

Certain edge cases (false positives) were found in the tests. These cases (<1%) are culled by more precise intersection tests before reporting a tile to be visible.
Few hotspots were found using a profiler so the order of intersection tests were slightly modified for a better performance.

Performance-wise the new implementation is few orders of magnitude slower than the original one. In a bigger picture the difference is not noticeable. I could see an increase of 5-15% time spent in TilePyramid::Update. Perhaps it's worth of discussion if the new tileCover should be a separate function and optional unless explicitly activated?

TODO

Investigate north orientation of map

mpulkki-mapbox · 2020-03-24T09:28:52Z

src/mbgl/util/tile_cover.cpp

+
+    const double numTiles = std::pow(2.0, z);
+    const double worldSize = Projection::worldSize(state.getScale());
+    const uint8_t minZoom = state.getPitch() <= (60.0 / 180.0) * M_PI ? z : 0;


This line controls if tiles from different zoom levels can be returned. Level-of-detail functionality is disabled when pitch is <= 60 degrees.

pozdnyakov · 2020-03-26T12:08:15Z

src/mbgl/util/bounding_volumes.hpp

+    // It is possible run only edge cases that were not covered in intersects()
+    IntersectionResult intersectsPrecise(const AABB& aabb, bool edgeCasesOnly = false) const;
+
+    inline const std::array<vec3, 8>& getPoints() const { return points; }


nit: inline is not needed

pozdnyakov · 2020-03-26T12:08:23Z

src/mbgl/util/bounding_volumes.hpp

+    IntersectionResult intersectsPrecise(const AABB& aabb, bool edgeCasesOnly = false) const;
+
+    inline const std::array<vec3, 8>& getPoints() const { return points; }
+    inline const std::array<vec4, 6>& getPlanes() const { return planes; }


pozdnyakov · 2020-03-26T12:09:37Z

src/mbgl/util/bounding_volumes.cpp

+
+    std::array<vec4, 6> frustumPlanes;
+
+    for (int i = 0; i < (int)frustumPlanePointIndices.size(); i++) {


nit: ++i
why not std::size_t

pozdnyakov · 2020-03-26T12:12:11Z

Looks great, but let @astojilj to take a look before merge

astojilj · 2020-03-26T12:29:05Z

Performance-wise the new implementation is few orders of magnitude slower than the original one. In a bigger picture the difference is not noticeable. I could see an increase of 5-15% time spent in TilePyramid::Update. Perhaps it's worth of discussion if the new tileCover should be a separate function and optional unless explicitly activated?

In case that this was measured using some benchmark, how much is "tile cover update" budget per frame affected by this on example mobile platform?

astojilj · 2020-03-31T09:32:33Z

src/mbgl/util/bounding_volumes.cpp

+        if (result == IntersectionResult::Separate) return result;
+    }
+
+    const std::array<vec3, 4> aabbPoints = {vec3{aabb.min[0], aabb.min[1], 0.0},


Could alloc and copy here get removed, and add methods to access min and max also for dim 2?

EDIT: For AABB we store min & max vectors and in addition, for every AABB we construct aabbPoints. Might help performance if allocate one only (whichever fits algorithm better) and reduce copies...

All these arrays are stack allocated so the only cost should be variable initialization.

Z-dimension is redundant as all tiles are on the xy-plane and leaving z out reduces the number of required intersection tests. This could be updated if we ever have a need for elevation.

Corner points of the aabb could be pre-generated but it would be premature optimization in this case. Intersection tests are performed once per aabb and moving the generation of points to constructor wouldn't remove any logic. Frustum::intersects also requires a w component whereas Frustum::intersectsPrecise does not.

astojilj · 2020-03-31T12:48:35Z

LGTM after documenting performance impact on one frame.

mpulkki-mapbox · 2020-04-03T10:44:20Z

Performance-wise the new implementation is few orders of magnitude slower than the original one. In a bigger picture the difference is not noticeable. I could see an increase of 5-15% time spent in TilePyramid::Update. Perhaps it's worth of discussion if the new tileCover should be a separate function and optional unless explicitly activated?

In case that this was measured using some benchmark, how much is "tile cover update" budget per frame affected by this on example mobile platform?

I profiled the code on my quite fast laptop in the following location: [INFO] {mbgl-glfw}[General]: Exit location: --lat="60.169900" --lon="24.943500" --zoom="16.064100" --bearing "43.857600". I chose few test runs describing the average performance of both new and original tileCover implementation. These numbers are not exact but they give a rough idea of the performance impact.

original:

new:

Tests were done by rendering a map 60fps for 10 seconds using "streets-v11" style. Time spent in TilePyramid::update increased ~10%. tileCover is also called from render_background_layer.cpp, but the profiler could not catch that. Frame budget used by TilePyramid::update during a 10s test run increases from ~2.7% to ~3.0% based on these observations. Impact on the overall tile budget is quite small.

mpulkki-mapbox marked this pull request as ready for review March 24, 2020 09:20

mpulkki-mapbox commented Mar 24, 2020

View reviewed changes

mpulkki-mapbox requested review from astojilj and pozdnyakov March 24, 2020 09:30

pozdnyakov approved these changes Mar 26, 2020

View reviewed changes

mpulkki-mapbox force-pushed the mpulkki_tilecover_lod branch 2 times, most recently from f0dc2a8 to 41a50ac Compare March 30, 2020 09:17

astojilj reviewed Mar 31, 2020

View reviewed changes

mpulkki-mapbox force-pushed the mpulkki_tilecover_lod branch 2 times, most recently from 8dbc84d to aab6608 Compare April 3, 2020 10:02

Refactor tileCover to support lod tiles

1c94e0e

mpulkki-mapbox force-pushed the mpulkki_tilecover_lod branch from aab6608 to 1c94e0e Compare April 3, 2020 10:24

astojilj approved these changes Apr 3, 2020

View reviewed changes

mpulkki-mapbox merged commit d0c103f into master Apr 6, 2020

mpulkki-mapbox deleted the mpulkki_tilecover_lod branch April 6, 2020 09:10

astojilj mentioned this pull request Jun 6, 2020

Combination of padding and pitch lead to unexpected results #16490

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor tile coverage to support level of detail #16335

Refactor tile coverage to support level of detail #16335

mpulkki-mapbox commented Mar 23, 2020 •

edited

Loading

mpulkki-mapbox Mar 24, 2020

pozdnyakov Mar 26, 2020

pozdnyakov Mar 26, 2020

pozdnyakov Mar 26, 2020

pozdnyakov commented Mar 26, 2020

astojilj commented Mar 26, 2020

astojilj Mar 31, 2020 •

edited

Loading

mpulkki-mapbox Mar 31, 2020 •

edited

Loading

astojilj commented Mar 31, 2020

mpulkki-mapbox commented Apr 3, 2020


		std::array<vec4, 6> frustumPlanes;

		for (int i = 0; i < (int)frustumPlanePointIndices.size(); i++) {

Refactor tile coverage to support level of detail #16335

Refactor tile coverage to support level of detail #16335

Conversation

mpulkki-mapbox commented Mar 23, 2020 • edited Loading

mpulkki-mapbox Mar 24, 2020

Choose a reason for hiding this comment

pozdnyakov Mar 26, 2020

Choose a reason for hiding this comment

pozdnyakov Mar 26, 2020

Choose a reason for hiding this comment

pozdnyakov Mar 26, 2020

Choose a reason for hiding this comment

pozdnyakov commented Mar 26, 2020

astojilj commented Mar 26, 2020

astojilj Mar 31, 2020 • edited Loading

Choose a reason for hiding this comment

mpulkki-mapbox Mar 31, 2020 • edited Loading

Choose a reason for hiding this comment

astojilj commented Mar 31, 2020

mpulkki-mapbox commented Apr 3, 2020

mpulkki-mapbox commented Mar 23, 2020 •

edited

Loading

astojilj Mar 31, 2020 •

edited

Loading

mpulkki-mapbox Mar 31, 2020 •

edited

Loading